-
Notifications
You must be signed in to change notification settings - Fork 193
Add ESP_LOG support in platform #403
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Update the platform logging to use the ESP_LOGx macros when MO_PLATFORM is set to MO_PLATFORM_ESPIDF.
|
Looking into this a bit more deeply, this isn't a great drop-in replacement because I also didn't previously realize that log levels were actually supported in @matth-x do you have any suggestions on the best way to modify the logging infrastructure here to cleanly insert |
|
We would also love to have this, as we then tunnel ESP-IDF logs into our web interface. Any way we can support to make this happen? |
|
Thanks @razvanphp, we do the same. Our current workaround is to use Either way, I may patch this to use |
|
@devunwired Thanks for proposing a better solution to this! In the current debug macros, the log levels handling is a pain and it will be improved in the next MO major version. The current possible workarounds are:
Neither of the workarounds are great. I would tend to leave it to the implementer to choose the workaround and make the log messages work on their platform. And otherwise, encourage the MOv2 upgrade. Do you agree, or do you think that the In MOv2, it will be possible to set the MicroOcpp/src/MicroOcpp/Debug.h Line 44 in f151cec
I see that the built-in log function makes the same mistake as in the current version and doesn't make use of the ESP logger library. Noted on my end! |
Replace the ESP_LOG macro with esp_log_write, which is a better replacement for printf style logging.
|
I just bumped this change to use
IMO, the callback mechanism makes sense for implementers using a platform that doesn't have built-in support or if you have custom processing to do on the log messages. Since the library has a platform layer with built-in support for ESP-IDF, then Note: There is also a |
|
Okay, agreed. Thanks for your improvement to MicroOCPP, Dave! |
Update the platform logging to use the
ESP_LOGxmacros whenMO_PLATFORMis set toMO_PLATFORM_ESPIDF.This allows log messages from the library to be managed properly alongside other application logs when using functions from the ESP-IDF logging library like
esp_log_level_set()to supress logs oresp_log_set_vprintf()to route them to a different destination.